Skip to content

feat: support subgroup aesthetic in geom_polygon for holes#305

Open
AviraL0013 wants to merge 5 commits intoanimint:masterfrom
AviraL0013:feature/polygon-subgroup-holes
Open

feat: support subgroup aesthetic in geom_polygon for holes#305
AviraL0013 wants to merge 5 commits intoanimint:masterfrom
AviraL0013:feature/polygon-subgroup-holes

Conversation

@AviraL0013
Copy link

@AviraL0013 AviraL0013 commented Mar 2, 2026

Problem

working on #252

When using isoband::isobands() to generate contour data for shapes with holes (like a donut/ring), animint2 was rendering two separate filled polygons instead of one polygon with a transparent hole. This made it impossible to correctly visualize Hi-C genomic cluster contours and similar real-world data.

Before (bug):
Screenshot 2026-03-02 042021

The root cause was a two-layer problem:

  1. R compiler: The subgroup aesthetic (added in ggplot2 3.2.0) was completely absent from GeomPolygon — not in default_aes, not detected by the compiler
  2. JS renderer: No fill-rule logic existed — SVG requires fill-rule: evenodd to render polygon holes correctly

Solution

R/geom-polygon.r

Added subgroup = NULL to default_aes so GeomPolygon recognizes the aesthetic.

R/geom-.r

In export_animint, added detection of subgroup aesthetic on polygon geoms and sets g$data_has_subgroup <- TRUE. This flag gets serialized into plot.json and read by the browser renderer.

inst/htmljs/animint.js

Modified eActions for grouped geoms to:

  • Detect g_info.data_has_subgroup
  • Group points by "subgroup" column using d3.nest()
  • Build separate SVG subpaths per subgroup, each closed with "Z"
  • Apply fill-rule: evenodd — standard SVG technique for holes

Key finding: ggplot_build renames data columns to the aesthetic name, so the TSV column is always "subgroup" regardless of the original variable name in the data.

After (fix):
Screenshot 2026-03-02 043049

Reproducer

library(animint2)
library(isoband)
library(data.table)

m <- matrix(c(0,0,0,0,0,0,
              0,1,1,1,1,0,
              0,1,0,0,1,0,
              0,1,0,0,1,0,
              0,1,1,1,1,0,
              0,0,0,0,0,0), 6, 6, byrow=TRUE)

res <- isoband::isobands(
  (1:ncol(m))/(ncol(m)+1),
  (nrow(m):1)/(nrow(m)+1),
  m, 0.5, 1.5)[[1]]

animint2dir(list(
  poly = ggplot() +
    geom_polygon(aes(x, y, group=1, subgroup=id),
                 data=as.data.table(res))
))

@tdhock Could you please take a look

TODO

  • Add testthat test with clickID/mouseMove
  • Add vignette example
  • Test edge case: more than 2 subgroups

@AviraL0013
Copy link
Author

AviraL0013 commented Mar 2, 2026

image

Sharing the latest output

@tdhock
Copy link
Collaborator

tdhock commented Mar 8, 2026

this is encouraging thanks.
but not the solution that I expected.
Doesn’t D3 provide a function for this?

@AviraL0013
Copy link
Author

AviraL0013 commented Mar 8, 2026 •
edited
Loading

Thanks for the feedback @tdhock! I investigated using d3.geo.path() as linked in the original issue.

Finding: The D3 v3 build bundled with animint2 (inst/htmljs/vendor/d3.v3.js) is v3.0.6 — a custom/partial build that does not include the d3.geo module. So d3.geo.path() is currently unavailable.

To use the d3.geo.path().projection(null) approach, we'd need to either:

  1. Upgrade the D3 vendor file to the full D3 v3.5.17 (final v3 release, backward-compatible API)
  2. Extract just the d3.geo.path functions from v3.5.17 and append them to the existing build

The implementation would then:

  • Group points by subgroup column using d3.nest()
  • Construct a GeoJSON Polygon object (first subgroup = exterior ring, subsequent subgroups = hole rings)
  • Render via d3.geo.path().projection(null) — the null projection treats coordinates as raw pixel values, which is what we want since the JS renderer applies scales.x() / scales.y() to the data coordinates before constructing the GeoJSON rings
  • D3 handles winding order and path construction internally, so no manual fill-rule: evenodd is needed

Which approach do you prefer — upgrading the full D3 v3 bundle, or extracting just the geo path functions?

Edit: On rechecking, d3.geo.path is actually present in the bundled d3.v3.js at line 6219 — no bundle changes needed. Will use d3.geo.path().projection(null) directly.

@tdhock
Copy link
Collaborator

tdhock commented Mar 10, 2026

thanks. please try whatever approach is simpler and easier to review?

@AviraL0013
Copy link
Author

AviraL0013 commented Mar 10, 2026

So Sorry for the confusion earlier @tdhock! I had incorrectly concluded that d3.geo.path was missing from the bundled d3.v3.js — I missed it on my first pass. After looking more carefully, it is actually present in the bundle.

So the simplest approach is to use d3.geo.path().projection(null) directly, no bundle changes needed. Will update the PR accordingly.

@AviraL0013
refactor: use d3.geo.path().projection(null) for polygon subgroup holes
1617eff 
- Replace manual SVG subpath concatenation with d3.geo.path()
- Construct GeoJSON Polygon from subgroup coordinate rings
- Keep fill-rule: evenodd as safety net for winding order
- Consolidate tests for easier review (215 -> 131 lines)
@AviraL0013
perf: only create geoPath when data_has_subgroup is true
64b0f3d
@AviraL0013
Copy link
Author

AviraL0013 commented Mar 10, 2026

D3 Geo Path Update Notes

Updated to use d3.geo.path().projection(null) directly — it's already available in the bundled d3.v3.js (line 6219), no D3 upgrade needed.

Changes

  • Construct a GeoJSON Polygon from subgroup coordinate rings and render via d3.geo.path().projection(null) (null = raw pixel coords)
  • geoPath created once, only when data_has_subgroup is true
  • fill-rule: evenodd kept as safety net for winding order
  • Consolidated tests (215 → 131 lines)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@AviraL0013 @tdhock